Add new FileIo CRT bindings #6439
Conversation
…shouldStream` to `uploadBufferDisabled`
L-Applin
added
the
api-surface-area-approved-by-team
|
|
|
||
| /** | ||
| * The estimated disk throughput in gigabits per second (Gbps). | ||
| * Only applied when {@code shouldStream} is true. |
There was a problem hiding this comment.
Looks like typo and should be {@code uploadBufferDisabled}
| * @param shouldStream whether to stream the file | ||
| * @return The builder for method chaining. | ||
| */ | ||
| Builder uploadBufferDisabled(Boolean shouldStream); |
There was a problem hiding this comment.
| Builder uploadBufferDisabled(Boolean shouldStream); | |
| Builder uploadBufferDisabled(Boolean uploadBufferDisabled); |
|
|
||
| /** | ||
| * The estimated disk throughput in gigabits per second (Gbps). | ||
| * Only applied when {@code shouldStream} is true. |
There was a problem hiding this comment.
Can we also mention in Java doc why shouldStream is referenced as uploadBufferDisabled in Java client?
| } | ||
|
|
||
| private static FileIoOptions resolveFileIoOptions(Builder builder) { | ||
| S3CrtFileIoConfiguration s3CrtFileIoConfiguration = builder.fileIoConfiguration; |
There was a problem hiding this comment.
Can builder.fileIoConfiguration; be null ? We might need to handle if null condition for builder.fileIoConfiguration;
| * <p> | ||
| * Notes: | ||
| * <ul> | ||
| * <li>Only supported on Linux for now.</li> |
There was a problem hiding this comment.
Is there a way we default it to fault if user sets this for non linux ?
| * </ul> | ||
| * for additional info https://man7.org/linux/man-pages/man2/openat.2.html | ||
| */ | ||
| public static final SdkAdvancedAsyncClientOption<Boolean> CRT_UPLOAD_FILE_DIRECT_IO = |
There was a problem hiding this comment.
Should we create a new Option class for CRT and put CRT specific option there?
| } | ||
|
|
||
| @Override | ||
| public <T> S3CrtAsyncClientBuilder putAdvancedOption(SdkAdvancedAsyncClientOption<T> option, T value) { |
There was a problem hiding this comment.
Should we have new option class for CRT like SdkAdvancedAsyncCrtClientOption , else all the options which are available on SdkAdvancedAsyncClientOption might appear as supported for CRT and vice versa.
Adding new CRT bindings to s3 crt async client.
Motivation and Context
New bindings were added in version
0.39.0for file operations.Modifications
0.39.0@SdkPublicApi) config class:S3CrtFileIoConfigurationS3CrtAsyncClientBuilderadvancedOptionsmethod in crt builder for the newSdkAdvancedAsyncClientOption.CRT_UPLOAD_FILE_DIRECT_IOS3NativeClientConfigurationmodified to include the newFileIoOptionfrom CRT